Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: remove swap findChildren #2077

Merged
merged 2 commits into from
Mar 7, 2025
Merged

Conversation

alessey
Copy link
Contributor

@alessey alessey commented Mar 4, 2025

What changed? Why?
Update Swap and SwapSettings components to have default children. Added toToken, fromToken, swappableTokens as top level props when children is undefined. Interface is unchanged when children are passed in.

  • Remove findChildren
  • Add default children
  • SwappableTokens, toToken, fromToken to Swap component, required when children is undefined
swap default swap with children swap with extra
image image image

Notes to reviewers

How has it been tested?

Copy link

vercel bot commented Mar 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
onchainkit-coverage ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 6, 2025 9:48pm
onchainkit-playground ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 6, 2025 9:48pm
onchainkit-routes ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 6, 2025 9:48pm

@alessey alessey force-pushed the alessey/remove-swap-findChildren branch from 26aa974 to e9360fd Compare March 4, 2025 20:31
@alessey alessey force-pushed the alessey/remove-swap-findChildren branch from e9360fd to ee37dc9 Compare March 4, 2025 21:03
@alessey alessey force-pushed the alessey/remove-swap-findChildren branch from ee37dc9 to c4c27e3 Compare March 4, 2025 21:09
@alessey alessey force-pushed the alessey/remove-swap-findChildren branch from c4c27e3 to fce4b4c Compare March 4, 2025 21:13
@alessey alessey force-pushed the alessey/remove-swap-findChildren branch from fce4b4c to 875db96 Compare March 4, 2025 21:14
@alessey alessey marked this pull request as ready for review March 4, 2025 21:44
@alessey alessey force-pushed the alessey/remove-swap-findChildren branch from 875db96 to 4d6f156 Compare March 5, 2025 14:44
maxSlippage: defaultMaxSlippage || FALLBACK_DEFAULT_MAX_SLIPPAGE,
}}
isSponsored={isSponsored}
headerLeftContent={<div>test</div>}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test intended?

Copy link
Contributor Author

@alessey alessey Mar 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yea, I will be removing these playground updates prior to merging, but wanted to make it easy to test the different permutations.

token={ethToken}
type="from"
<div className="relative flex flex-col gap-6">
<h2>Swap Default</h2>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since you're adding a default here, should we remove SwapDefault from the dropdown?

image

brendan-defi
brendan-defi previously approved these changes Mar 5, 2025
dschlabach
dschlabach previously approved these changes Mar 6, 2025
@alessey alessey merged commit c03cf27 into main Mar 7, 2025
16 checks passed
@alessey alessey deleted the alessey/remove-swap-findChildren branch March 7, 2025 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants